feat: add funnelIds to remoteConfig and handle development defaults #2899
Conversation
ilasw
requested review from
rebelchris,
omBratteng,
sshanzel,
nensidosari and
AmarTrebinjac
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Development Mode Funnel ID Retrieval Bug
In development mode (!isProd && !isTest), the resolveDynamicFunnelId function's logic for returning static funnel IDs is flawed. If GROWTHBOOK_API_CONFIG_CLIENT_KEY is set, remoteConfig.vars attempts to fetch from GrowthBook instead of providing the intended hardcoded development defaults. This leads to two issues:
- If GrowthBook doesn't return
funnelIds, the function falls back to 'off' instead of the expected static values, contradicting the code comment. - Accessing
remoteConfig.varsbeforeremoteConfig.init()completes can throw a "remote config not initialized" error, creating a race condition during application startup whenGROWTHBOOK_API_CONFIG_CLIENT_KEYis present.
src/routes/boot.ts#L789-L794
Lines 789 to 794 in c684915
Comment bugbot run to trigger another review on this PR
Was this report helpful? Give feedback by reacting with 👍 or 👎
| @@ -28,6 +28,7 @@ type RemoteConfigValue = { | |||
| paddleIps: string[]; | |||
| paddleTestDiscountIds: string[]; | |||
| paddleProductIds: Partial<Record<PurchaseType, string>>; | |||
| funnelIds: Record<string, string>; | |||
There was a problem hiding this comment.
Can we make this typed web_funnel_id, onboarding_funnel_id, I don't think we will introduce many different funnel types as much + also they need other changes so better to have it typed
There was a problem hiding this comment.
| funnelIds: Record<string, string>; | |
| funnelIds: Partial<{ | |
| web_funnel_id: string, | |
| onboarding_funnel_id: string | |
| }>; |
No description provided.